-
Notifications
You must be signed in to change notification settings - Fork 676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use separate nodeProjectDir for each subproject #2680
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to make sense, I wish I knew more to weigh in one way or the other, hopefully someone else gives a +1!
This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the [email protected] mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution! |
This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see no reason for not doing this? Fixes a bug..
@tylerbertrand Will you resolve the conflicts, then me or @epugh will get it merged? |
thanks for reminding us @janhoy |
@janhoy Sure, sounds good |
Using a shared nodeProjectDir caused multiple nodeSetup tasks to step on each others' toes. The com.github.node-gradle.node plugin doesn't fully support reusing nodeProjectDir, and each nodeSetup task will clean up the existing nodeProjectDir before unpacking the node installation into it. This can result in tasks seeing issues when trying to use npm/node while a nodeSetup task from another project is cleaning it up. use nodeProjectDir instead of rootNodeDir
3043002
to
760348e
Compare
Using a shared nodeProjectDir caused multiple nodeSetup tasks to step on each others' toes. The com.github.node-gradle.node plugin doesn't fully support reusing nodeProjectDir, and each nodeSetup task will clean up the existing nodeProjectDir before unpacking the node installation into it. This can result in tasks seeing issues when trying to use npm/node while a nodeSetup task from another project is cleaning it up. use nodeProjectDir instead of rootNodeDir --------- Co-authored-by: Eric Pugh <[email protected]> (cherry picked from commit 388101f)
Thanks for the contribution @tylerbertrand! |
I'm going to move the CHANGES.txt entry from "Bug fix" into the "Other" category because Solr itself does not have a bug. Also note the Lucene project uses "GITHUB#1234" pattern; we might want to do similarly. It's more future-proof as some references are to issues not PRs specifically. I'm making this change as well. |
Thanks @dsmiley ... I guess I was thinking as developer of Solr versus a user of Solr! And Changes.txt is meant for the user primarily! |
Description
Sometimes,
NpmTask
s are failing with being unable to find files/commands undernpmProjectDir
. Here's an example of such a failure. The failing tasks always execute after thenodeSetup
task for that project, so in theory, everything should be in place.However, in this case, multiple subprojects share the same
npmProjectDir
. The way thegradle-node-plugin
works, having multiple projects use the samenpmProjectDir
doesn't save work, and can actually cause timing issues like the failures we are seeing. This is because eachnodeSetup
task clears out the old directory before it unpacks it again. In the Timeline view of the same Build Scan linked above, we can see that the:solr:solr-ref-guide:downloadAntoraCli
task (anNpmTask
), is executing at the same time as:solr:packaging:nodeSetup
, anddownloadAntoraCli
can't findnode
because it's been cleared out by another suproject'snodeSetup
task.Solution
This PR updates each subproject to use its own individual
nodeProjectDir
to avoid these timing issues. Separating these out doesn't result innode
being downladed twice - despite unpacking the same version multiple times, it's only ever downloaded once. So no extra work is being done, since sharing the samenodeProjectDir
doesn't save work in the first place, as explained above.Tests
No new tests were added, and no existing tests were modified, but the
check
andintegrationTests
tasks consistently pass when run locally with these changes.Checklist
Please review the following and check all that apply:
main
branch../gradlew check
.